Skip to content

Restore cider--display-interactive-eval-result overlays for compilation errors #3492

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Sep 30, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Sep 29, 2023

Fixes #3487

  • cider-interactive-eval-handler: Check if the given exception's phase is ignored, and if so, display the overlay.
    • The general idea is, we either display the overlay or *cider-error* (not both, not none)
  • Reuse the same computation ("is the phase ignored?") and if truthy, we avoid jumping to the compilation line (because for compilation errors, it's always the current one, while nrepl while tell us 0:0, which we cannot detect as spurious, client-side)

I've tested this out locally for all cases, and it works nicely.

No changelog is needed.

Cheers - V

@vemv vemv requested a review from bbatsov September 29, 2023 19:36
(when-let ((conn (with-current-buffer buffer
(cider-current-repl))))
(when (cider-nrepl-op-supported-p "analyze-last-stacktrace" conn)
(when-let* ((result (nrepl-send-sync-request (thread-last (map-merge 'list
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be up to 2 analyze-last-stacktrace requests during the same interaction (one async in defun cider-default-err-op-handler, one sync in here).

About 1mo ago I had tried doing this sort of thing in just 1 async call, but was overly complex.

For avoiding any unnecessary costs, we can introduce some caching, cider-nrepl side. The cache would have a size of 1 (since one normally is only interested in analyzing the last exception - older exceptions are uninteresting / unlikely to be re-queried)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this out and while our stacktrace analysis isn't blazing-fast, runtimes in the 20-40ms range don't seem worth caching.

cider-eval.el Outdated
@@ -774,6 +775,19 @@ REPL buffer. This is controlled via
(cider--make-fringe-overlay (point)))
(scan-error nil)))))

(defun cider--error-phase-of-latest-exception (buffer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably last is a better term than latest for this. Or most-recent.

@bbatsov
Copy link
Member

bbatsov commented Sep 30, 2023

The changes look good to me.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error notifications missing
2 participants